Add ConfigureHostOptions implemetation #49502

I-SER-I · 2021-03-11T20:34:05Z

dotnet-issue-labeler · 2021-03-11T20:34:10Z

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

ghost · 2021-03-11T20:34:13Z

Tagging subscribers to this area: @eerhardt, @maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details

Author:	I-SER-I
Assignees:	-
Labels:	`area-Extensions-Hosting`, `new-api-needs-documentation`
Milestone:	-

src/libraries/Microsoft.Extensions.Hosting/src/HostingHostBuilderExtensions.cs

maryamariyan

Thanks for the PR @I-SER-I , could you please add tests for the new API under src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostBuilderTests.cs?

src/libraries/Microsoft.Extensions.Hosting/ref/Microsoft.Extensions.Hosting.cs

maryamariyan · 2021-03-18T00:05:21Z

src/libraries/Microsoft.Extensions.Hosting/src/HostingHostBuilderExtensions.cs

+        /// <returns>The same instance of the <see cref="IHostBuilder"/> for chaining.</returns>
+        public static IHostBuilder ConfigureHostOptions(this IHostBuilder hostBuilder, Action<HostOptions> configureDelegate)
+        {
+            return hostBuilder.ConfigureServices((context, collection) => collection.Configure(configureDelegate));


nit: you could be using the other overload here for simplicity, since no context is passed around

Suggested change

return hostBuilder.ConfigureServices((context, collection) => collection.Configure(configureDelegate));

return hostBuilder.ConfigureServices(collection => collection.Configure(configureDelegate));

maryamariyan · 2021-03-18T00:13:22Z

src/libraries/Microsoft.Extensions.Hosting/ref/Microsoft.Extensions.Hosting.cs

@@ -42,6 +42,7 @@ public static partial class HostingHostBuilderExtensions
        public static Microsoft.Extensions.Hosting.IHostBuilder ConfigureLogging(this Microsoft.Extensions.Hosting.IHostBuilder hostBuilder, System.Action<Microsoft.Extensions.Hosting.HostBuilderContext, Microsoft.Extensions.Logging.ILoggingBuilder> configureLogging) { throw null; }
        public static Microsoft.Extensions.Hosting.IHostBuilder ConfigureLogging(this Microsoft.Extensions.Hosting.IHostBuilder hostBuilder, System.Action<Microsoft.Extensions.Logging.ILoggingBuilder> configureLogging) { throw null; }
        public static Microsoft.Extensions.Hosting.IHostBuilder ConfigureServices(this Microsoft.Extensions.Hosting.IHostBuilder hostBuilder, System.Action<Microsoft.Extensions.DependencyInjection.IServiceCollection> configureDelegate) { throw null; }
+        public static Microsoft.Extensions.Hosting.IHostBuilder ConfigureHostOptions(this Microsoft.Extensions.Hosting.IHostBuilder hostBuilder, System.Action<Microsoft.Extensions.Hosting.HostOptions> configureDelegate) { throw null; }


Similar to how we have two ConfigureLogging overloads:

runtime/src/libraries/Microsoft.Extensions.Hosting/ref/Microsoft.Extensions.Hosting.cs

Lines 42 to 43 in 689c492

public static Microsoft.Extensions.Hosting.IHostBuilder ConfigureLogging(this Microsoft.Extensions.Hosting.IHostBuilder hostBuilder, System.Action<Microsoft.Extensions.Hosting.HostBuilderContext, Microsoft.Extensions.Logging.ILoggingBuilder> configureLogging) { throw null; }

public static Microsoft.Extensions.Hosting.IHostBuilder ConfigureLogging(this Microsoft.Extensions.Hosting.IHostBuilder hostBuilder, System.Action<Microsoft.Extensions.Logging.ILoggingBuilder> configureLogging) { throw null; }

ConfigureHostOptions could have another overload as well with the following signature:

Suggested change

public static Microsoft.Extensions.Hosting.IHostBuilder ConfigureHostOptions(this Microsoft.Extensions.Hosting.IHostBuilder hostBuilder, System.Action<Microsoft.Extensions.Hosting.HostOptions> configureDelegate) { throw null; }

public static Microsoft.Extensions.Hosting.IHostBuilder ConfigureHostOptions(this Microsoft.Extensions.Hosting.IHostBuilder hostBuilder, System.Action<Microsoft.Extensions.Hosting.HostBuilderContext, Microsoft.Extensions.Hosting.HostOptions> configureOptions) { throw null; }

public static Microsoft.Extensions.Hosting.IHostBuilder ConfigureHostOptions(this Microsoft.Extensions.Hosting.IHostBuilder hostBuilder, System.Action<Microsoft.Extensions.Hosting.HostOptions> configureOptions) { throw null; }

eerhardt · 2021-04-16T15:13:06Z

@I-SER-I - Thanks for your help here. Are you interested in progressing this PR? If not, we can get someone else to take it forward.

I-SER-I · 2021-04-16T19:12:47Z

@eerhardt Sorry for the delay). I'll do it this week

IEvangelist · 2021-04-16T20:23:22Z

@eerhardt Eric Erhardt FTE Sorry for the delay). I'll do it this week

Hi @I-SER-I - I put together a PR #51410 that would close this one. I didn't realize that you were still working on this.

I-SER-I · 2021-04-16T22:53:24Z

@eerhardt Eric Erhardt FTE Sorry for the delay). I'll do it this week

Hi @I-SER-I - I put together a PR #51410 that would close this one. I didn't realize that you were still working on this.

No problem)

IEvangelist · 2021-04-17T03:15:52Z

@eerhardt
        Eric Erhardt
        FTE Eric Erhardt FTE Sorry for the delay). I'll do it this week
Hi @I-SER-I - I put together a PR #51410 that would close this one. I didn't realize that you were still working on this.
No problem)

Hey @I-SER-I - I think we should fix the merge conflicts in this PR, and update it with a few suggestions. Similar to what was done #51410. What do you say?

IEvangelist

Let's add the unit tests from https://github.com/dotnet/runtime/pull/51410/files#diff-030ac8a3306b8411705fab0423f5bbd96e0e7efa34ee6671db747c8d6cd8a27e here

IEvangelist · 2021-04-17T03:17:36Z

src/libraries/Microsoft.Extensions.Hosting/src/HostingHostBuilderExtensions.cs

+        /// <summary>
+        /// Adds a delegate for configuring the host settings. This can be called multiple times and
+        /// the results will be additive.
+        /// </summary>
+        /// <param name="hostBuilder">The <see cref="IHostBuilder"/> to configure.</param>
+        /// <param name="configureDelegate">The delegate for the <see cref="HostOptions"/>.</param>
+        /// <returns>The same instance of the <see cref="IHostBuilder"/> for chaining.</returns>
+        public static IHostBuilder ConfigureHostOptions(this IHostBuilder hostBuilder, Action<HostOptions> configureDelegate)
+        {
+            return hostBuilder.ConfigureServices((context, collection) => collection.Configure(configureDelegate));
+        }


Suggested change

/// <summary>

/// Adds a delegate for configuring the host settings. This can be called multiple times and

/// the results will be additive.

/// </summary>

/// <param name="hostBuilder">The <see cref="IHostBuilder"/> to configure.</param>

/// <param name="configureDelegate">The delegate for the <see cref="HostOptions"/>.</param>

/// <returns>The same instance of the <see cref="IHostBuilder"/> for chaining.</returns>

public static IHostBuilder ConfigureHostOptions(this IHostBuilder hostBuilder, Action<HostOptions> configureDelegate)

{

return hostBuilder.ConfigureServices((context, collection) => collection.Configure(configureDelegate));

}

/// <summary>

/// Adds a delegate for configuring the <see cref="HostOptions"/> of the <see cref="IHost"/>.

/// </summary>

/// <param name="hostBuilder">The <see cref="IHostBuilder" /> to configure.</param>

/// <param name="configureOptions">The delegate for configuring the <see cref="HostOptions"/>.</param>

/// <returns>The same instance of the <see cref="IHostBuilder"/> for chaining.</returns>

public static IHostBuilder ConfigureHostOptions(this IHostBuilder hostBuilder, Action<HostBuilderContext, HostOptions> configureOptions)

{

return hostBuilder.ConfigureServices(

(context, collection) => collection.Configure<HostOptions>(options => configureOptions(context, options)));

}

/// <summary>

/// Adds a delegate for configuring the <see cref="HostOptions"/> of the <see cref="IHost"/> instance

/// related to th.

/// </summary>

/// <param name="hostBuilder">The <see cref="IHostBuilder" /> to configure.</param>

/// <param name="configureOptions">The delegate for configuring the <see cref="HostOptions"/>.</param>

/// <returns>The same instance of the <see cref="IHostBuilder"/> for chaining.</returns>

public static IHostBuilder ConfigureHostOptions(this IHostBuilder hostBuilder, Action<HostOptions> configureOptions)

{

return hostBuilder.ConfigureServices(collection => collection.Configure(configureOptions));

}

eerhardt · 2021-04-19T19:01:15Z

Superseded by #51410.

Thanks for the contribution here @I-SER-I.

Add ConfigureHostOptions implemetation

d707962

dotnet-issue-labeler bot added area-Extensions-Hosting new-api-needs-documentation labels Mar 11, 2021

I-SER-I added 2 commits March 11, 2021 23:52

Fix ref type name missing

001a6ce

Fix arguments

80f8163

davidfowl reviewed Mar 12, 2021

View reviewed changes

src/libraries/Microsoft.Extensions.Hosting/src/HostingHostBuilderExtensions.cs Outdated Show resolved Hide resolved

Fix ref and implementation

689c492

davidfowl requested a review from maryamariyan March 17, 2021 23:25

maryamariyan reviewed Mar 17, 2021

View reviewed changes

src/libraries/Microsoft.Extensions.Hosting/ref/Microsoft.Extensions.Hosting.cs Show resolved Hide resolved

maryamariyan reviewed Mar 18, 2021

View reviewed changes

maryamariyan mentioned this pull request Mar 18, 2021

Add API to make configuring the HostOptions easier #48743

Closed

maryamariyan added the needs author feedback label Mar 18, 2021

maryamariyan self-assigned this Mar 18, 2021

IEvangelist mentioned this pull request Apr 16, 2021

Add ConfigureHostOptions extension method on IHostBuilder #51410

Merged

IEvangelist reviewed Apr 17, 2021

View reviewed changes

eerhardt closed this in #51410 Apr 19, 2021

ghost locked as resolved and limited conversation to collaborators May 19, 2021

karelz added this to the 6.0.0 milestone May 20, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ConfigureHostOptions implemetation #49502

Add ConfigureHostOptions implemetation #49502

I-SER-I commented Mar 11, 2021 •

edited

Loading

dotnet-issue-labeler bot commented Mar 11, 2021

ghost commented Mar 11, 2021

maryamariyan left a comment

maryamariyan Mar 18, 2021

maryamariyan Mar 18, 2021 •

edited by IEvangelist

Loading

eerhardt commented Apr 16, 2021

I-SER-I commented Apr 16, 2021

IEvangelist commented Apr 16, 2021

I-SER-I commented Apr 16, 2021

IEvangelist commented Apr 17, 2021

IEvangelist left a comment

IEvangelist Apr 17, 2021

eerhardt commented Apr 19, 2021

	return hostBuilder.ConfigureServices((context, collection) => collection.Configure(configureDelegate));
	return hostBuilder.ConfigureServices(collection => collection.Configure(configureDelegate));

	public static Microsoft.Extensions.Hosting.IHostBuilder ConfigureLogging(this Microsoft.Extensions.Hosting.IHostBuilder hostBuilder, System.Action<Microsoft.Extensions.Hosting.HostBuilderContext, Microsoft.Extensions.Logging.ILoggingBuilder> configureLogging) { throw null; }
	public static Microsoft.Extensions.Hosting.IHostBuilder ConfigureLogging(this Microsoft.Extensions.Hosting.IHostBuilder hostBuilder, System.Action<Microsoft.Extensions.Logging.ILoggingBuilder> configureLogging) { throw null; }

Add ConfigureHostOptions implemetation #49502

Add ConfigureHostOptions implemetation #49502

Conversation

I-SER-I commented Mar 11, 2021 • edited Loading

dotnet-issue-labeler bot commented Mar 11, 2021

ghost commented Mar 11, 2021

maryamariyan left a comment

Choose a reason for hiding this comment

maryamariyan Mar 18, 2021

Choose a reason for hiding this comment

maryamariyan Mar 18, 2021 • edited by IEvangelist Loading

Choose a reason for hiding this comment

eerhardt commented Apr 16, 2021

I-SER-I commented Apr 16, 2021

IEvangelist commented Apr 16, 2021

I-SER-I commented Apr 16, 2021

IEvangelist commented Apr 17, 2021

IEvangelist left a comment

Choose a reason for hiding this comment

IEvangelist Apr 17, 2021

Choose a reason for hiding this comment

eerhardt commented Apr 19, 2021

I-SER-I commented Mar 11, 2021 •

edited

Loading

maryamariyan Mar 18, 2021 •

edited by IEvangelist

Loading